fix: Fix usage of SearchableToolset with Agent when selecting a subset of tools to be active#11564
Conversation
…en using SearchableToolset
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
…ols is being restricted
Co-authored-by: Julian Risch <julian.risch@deepset.ai>
…. Remove .reset() since no longer needed.
|
@julian-risch based on our convo offline I've gone ahead and updated this PR to create a shallow copy of Toolset at Agent runtime using a new |
julian-risch
left a comment
There was a problem hiding this comment.
Code changes look good to me! 👍 The PR description is outdated now though. It still refers to reset(), which is no longer part of the code changes. Best to update the PR description, at least remove what is no longer valid so that it doesn't cause confusion. From my side, there is no need to describe the spawn() approach in the PR description now. The release note looks good.
Related Issues
Proposed Changes:
Before trying to restrict a
SearchableToolsetto a subset of its catalog by name at Agent runtime did not work:Before:
ValueError: The following tool names are not valid: {'get_weather', 'get_stock_price'}. Valid tool names are: {'search_tools'}SearchableToolsetonly exposed itssearch_toolsbootstrap tool (not its catalog) when flattened, so catalog tools couldn't be selected by name.tools=["search_tools"]instead would have collapsed the toolset into a single item list of justsearch_toolswhich disabled discovery.After: the
SearchableToolsetis kept intact, and when a user restricts the tools toget_weatherandget_stock_pricesearch and lazy-loading still work over the restricted subset.How this is fixed: instead of flattening the toolset into a static list of tools, the
Agentnow keeps theToolsetintact and applies the name selection to it:Toolset.get_selectable_tools().SearchableToolsetoverrides this to return its entire catalog, so catalog tools are now resolvable by name (previously only itssearch_toolstool was, since iterating the toolset only exposes that bootstrap tool plus already-discovered tools).list[Tool], theAgentregisters the requested names as a per-run selection on the liveToolset(viaToolset._selected_tool_names) and keeps that object as the run's tools.Agentre-reads its tools each step, theToolsetyields only the selected tools. ForSearchableToolsetthis means thesearch_toolstool is still exposed and search/discovery is restricted to the selected catalog tools — so search and lazy-loading keep working over the subset instead of being disabled.Changes Made:
Toolsetgained two methods:get_selectable_tools()returns every tool available for name-basedselection (ignoring any active selection restriction, e.g. the one used by
SearchableToolset), andspawn().SearchableToolsetnow exposes its full catalog of tools viaSearchableToolset.get_selectable_tools().Agent.run(tools=["tool_a", "tool_b"])now resolves correctly when aSearchableToolsetis configured. Previously theSearchableToolsetwas flattened into a single-itemlist (just its search tool), which broke its search and lazy-loading behavior.
How did you test it?
New tests
Notes for the reviewer
I did check locally that these changes work with our existing MCPToolset integration without any need for changes there.
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:and added!in case the PR includes breaking changes.